Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8e8184156
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| relative: getReferencePath("relative"), | ||
| remote, | ||
| remoteWithBranch, | ||
| remoteCanonical, | ||
| name, |
There was a problem hiding this comment.
Accept $canonical placeholder in reference snippets
The API docs for ReferenceSnippetDescriptor state that $canonical is a supported placeholder, but the substitution map only defines remoteCanonical (and subsituteSnippet throws on unknown variables). If a user writes a snippet using the documented $canonical placeholder, the reference modifier will throw “Unknown snippet variable: canonical” and fail. Either add a canonical alias in ReferenceStage or update the docs to match the implementation. See docs in packages/common/src/types/command/PartialTargetDescriptor.types.ts lines 423-426.
Useful? React with 👍 / 👎.
| const branchOrSha = isCanonical ? headSha : (defaultBranch ?? "main"); | ||
|
|
There was a problem hiding this comment.
Avoid defaulting to 'main' when default branch unknown
getRemoteGitPath falls back to "main" whenever origin/HEAD can’t be resolved. In repos whose default branch isn’t main (eg master, trunk, or any custom branch), this generates reference links that 404, even though the repo is otherwise valid. This occurs whenever git symbolic-ref --short refs/remotes/origin/HEAD fails (eg no remote HEAD set or remotes not fetched). Consider falling back to headSha or returning undefined instead of hardcoding main.
Useful? React with 👍 / 👎.
just a local experiment I've been running. pushing so I can use from another computer and in case anyone wants to look. working well for me so far
can try locally by installing extension from this PR and using https://github.com/cursorless-dev/cursorless-talon-staging/tree/pr/3133 cursorless-talon branch